Skip to content

fs/aio: rework lio_listio() and fix AIO crashes and POSIX conformance - #20107

Merged
acassis merged 13 commits into
apache:masterfrom
xiaoxiang781216:upstream-aio
Sep 14, 2026
Merged

acassis merged 13 commits into
apache:masterfrom
xiaoxiang781216:upstream-aio

Conversation

@xiaoxiang781216

Copy link
Copy Markdown
Contributor

Summary

  • Rework lio_listio() to link all requests of a batch into a list before submitting any I/O; aio_signal() then removes each completed node under aio_lock() and notifies the caller only when the list becomes empty. This fixes the thread-unsafe "submit first, set up notification state later" ordering of the old implementation.
  • Fix a family of crashes around the new lio_link machinery: uninitialized/overwritten list nodes (use list_clear_node() for non-batch operations, call the new aio_read_internal()/aio_write_internal() from lio_listio() to preserve list membership), a NULL-aiocbp dereference when no I/O could be queued, and an invalid list_delete() for failed submissions in LIO_WAIT mode.
  • Fix aioc use-after-free: the I/O workers decanted (freed) the container before signaling completion; aioc_decant() now runs after aio_signal().
  • Fix aio_cancel(): endless loop when cancelling already-running I/O, and missing EBADF validation of the file descriptor (file_get()/file_put()).
  • Align aio_read()/aio_write()/aio_error() return values with POSIX: -1 + errno = EINVAL for rejected requests (also retrievable via aio_error()), but 0 with the error reported through aio_error() for a bad file descriptor.
  • aio_suspend() now re-checks the completion list after every wakeup so a SIGPOLL from unrelated AIO no longer causes a spurious return, and the timeout is honored across wakeups.
  • Reject a NULL aiocbp in aio_fsync() (POSIX Issue 6 removed the NULL special case).
  • Make the lio_listio() prototype match POSIX (restrict qualifiers, unnamed parameters).
  • Add a configurable CONFIG_FS_AIO_LISTIO_MAX (default 10), validate nent against {AIO_LISTIO_MAX} in lio_listio(), and report it via sysconf(_SC_AIO_LISTIO_MAX).
  • Move lio_listio.c from libs/libc/aio to fs/aio so the whole AIO implementation lives in one directory.

Impact

  • All changes are confined to fs/aio/, libs/libc/aio/, include/aio.h, include/limits.h, libs/libc/libc.csv and libs/libc/unistd/lib_sysconf.c; no new dependencies.
  • struct aiocb layout changes (the unused aio_priv field is replaced by lio_link/lio_sigevent/lio_sigwork) — ABI-affecting for out-of-tree users of include/aio.h, which is why this is submitted as one series.
  • lio_listio() prototype gains restrict qualifiers per POSIX; existing callers compile unchanged.

Testing

  • sim:nsh with CONFIG_FS_AIO=y, CONFIG_TESTING_OSTEST=y, CONFIG_TESTING_OSTEST_AIO=y: build is warning-free; the full ostest run exits with status 0 and the AIO test reports all 7 cases (poll, LIO_WAIT, aio_suspend, individual signals, list completion signal, cancel by aiocb, cancel by fd) successful:
user_main: AIO test
AIO test case 1: Poll for transfer complete
...
AIO test case 7:Cancel I/O by file descriptor
  aio_cancel return 1
  ...
aio_test: Test completed successfully
ostest_main: Exiting with status 0
  • tools/checkpatch.sh -c -u -m -g passes for the whole series.

@github-actions github-actions Bot added Area: File System File System issues Size: L The size of the change in this PR is large labels Sep 10, 2026
@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown

MemBrowse Memory Report

No memory changes detected for:

acassis
acassis previously approved these changes Sep 10, 2026
Comment thread fs/aio/aio_cancel.c
@raiden00pl

Copy link
Copy Markdown
Member

intel64 LTP pass with this PR and #20112

raiden00pl
raiden00pl previously approved these changes Sep 11, 2026
@raiden00pl

raiden00pl commented Sep 11, 2026

Copy link
Copy Markdown
Member

@xiaoxiang781216 ltp_interfaces_lio_listio_2_1 test case for rv-virt/citest failed

jerpelea
jerpelea previously approved these changes Sep 11, 2026
lio_listio() submits I/O through the internal aio_read/aio_write
helpers and is only built when CONFIG_FS_AIO is enabled.  Keeping it in
libs/libc splits one subsystem across two directories and forces fs/aio
to export internal interfaces to the libc build.

Move the file (and its two build system entries) from libs/libc/aio to
fs/aio so that the whole AIO implementation lives in one place.

Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Previously, lio_listio() called aio_read()/aio_write() to submit the
I/O and only then initialized the per-request notification state
(aio_priv based), so a worker thread could complete an operation before
that state was set up (thread-unsafe), and the completion notification
hijacked the per-request sigevent machinery.

Rework the implementation: lio_listio() now links every aiocb of the
batch into a list (lio_link) before any I/O is submitted.  When an
operation completes, aio_signal() removes its node from the list under
aio_lock() and delivers the lio_listio completion notification only
when the list becomes empty.  The unused aio_priv field is replaced by
the lio_link/lio_sigevent/lio_sigwork fields in struct aiocb.

Co-developed-by: wushenhui <wushenhui@xiaomi.com>
Signed-off-by: wushenhui <wushenhui@xiaomi.com>
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
When lio_listio() is called with LIO_NOWAIT and a non-NULL sig, and no
I/O could be queued (or all entries are LIO_NOP/NULL), the completion
notification dereferences a NULL aiocbp picked from an empty iteration,
crashing nxsig_notification().

Scan the list for any non-NULL entry before delivering the
notification, and skip it entirely when the list contains only NULL
entries.

Signed-off-by: zhengyu16 <zhengyu16@xiaomi.com>
When a queued operation fails immediately (bad fd, EINVAL, or a failed
aio_read/aio_write submission), lio_listio() unconditionally deleted
the aiocbp from the request list.  In LIO_WAIT mode (or when no sig was
requested) the lio_link nodes were never linked into the list, so
list_delete() corrupted memory and crashed.

Only unlink the node when it was actually linked, i.e. when
mode == LIO_NOWAIT and a sigevent was provided.

Signed-off-by: tengshuangshuang <tengshuangshuang@xiaomi.com>
POSIX declares lio_listio() as:

  int lio_listio(int, struct aiocb *restrict const [restrict], int,
                 struct sigevent *restrict);

Update the prototype in include/aio.h (and the implementation and
libc.csv entry) accordingly, and drop the parameter names from the
other aio_* prototypes for consistency.

Signed-off-by: guoshichao <guoshichao@xiaomi.com>
lio_listio() never validated 'nent' against {AIO_LISTIO_MAX}, so a
batch larger than the documented limit was silently accepted, and the
hard-coded _POSIX_AIO_LISTIO_MAX value of 2 was too small for real
workloads (LTP uses 10 entries per call).

Add the FS_AIO_LISTIO_MAX Kconfig option (default 10), use it for
_POSIX_AIO_LISTIO_MAX in include/limits.h, validate 'nent' in
lio_listio(), and report the limit through sysconf(_SC_AIO_LISTIO_MAX).

Signed-off-by: tengshuangshuang <tengshuangshuang@xiaomi.com>
aioc_decant() frees the AIO container and detaches the aiocbp.  The
I/O workers (aio_read_worker, aio_write_worker, aio_fsync_worker)
called it before signaling completion, so aio_signal() and any code
touching the container afterwards ran on freed memory.  Additionally,
if the caller closed the file early the detached container could be
reused with a stale file reference.  Move aioc_decant() to after
aio_signal() and use aioc->aioc_aiocbp directly in the workers.

aio_cancel() also had two problems: with no aiocbp it looped over
g_aio_pending with a do/while that skipped the list re-entry check, so
a failed work_cancel() on an already running I/O caused an endless
loop; and an invalid fildes only checked 'fildes < 0' instead of
validating the descriptor, so a closed fd was not reported as EBADF.
Use a for-loop that always advances and validate the descriptor with
file_get()/file_put().

Co-developed-by: wushenhui <wushenhui@xiaomi.com>
Signed-off-by: wushenhui <wushenhui@xiaomi.com>
Signed-off-by: tengshuangshuang <tengshuangshuang@xiaomi.com>
Per POSIX, aio_read() and aio_write() must return -1 and set errno to
EINVAL when the request cannot be queued (aio_reqprio < 0,
aio_offset < 0), and the error must also be retrievable via
aio_error().  Conversely, when queuing fails with a bad file
descriptor, the error belongs to the asynchronous operation: the
functions must return 0 and report EBADF through aio_error().

- Merge the offset/reqprio checks and return ERROR with errno set,
  after storing the result in aio_result for aio_error().
- Drop the aio_fildes < 0 early return: a closed descriptor is now
  caught by fcntl()/aio_queue() and reported through aio_result with
  the function returning OK.
- aio_error(): report -EINVAL (failed validation) through errno
  instead of returning it as an error value.

Signed-off-by: tengshuangshuang <tengshuangshuang@xiaomi.com>
aio_suspend() checked the completion status once and then performed a
single sigtimedwait().  Any SIGPOLL delivered by an unrelated AIO
operation (one not referenced by 'list') woke the caller even though
none of the awaited requests had completed, and with a timeout the
remaining wait time was not preserved either.

Re-check the completion status after every wakeup and continue
waiting, recomputing the remaining time from the absolute deadline so
that the full timeout is honored.

Signed-off-by: wushenhui <wushenhui@xiaomi.com>
aio_fsync() never initialized aiocbp->lio_link, but the reworked
aio_signal() tests list_in_list(&lio_link) on every completion.  With
an uninitialized (or zero-filled) lio_link the behavior was
unpredictable; initialize the node so standalone fsync operations are
self-consistent.

Signed-off-by: tengshuangshuang <tengshuangshuang@xiaomi.com>
lio_listio() links each aiocbp->lio_link into its batch list before
submitting the I/O, but submitted the operations through the public
aio_read()/aio_write(), which re-initialized lio_link and destroyed
the list membership.  With an aiocb pre-filled with garbage (as in
ostest), the completion path then walked an invalid list.

Extract aio_read_internal()/aio_write_internal() that skip the
lio_link setup; aio_read()/aio_write() initialize lio_link (and
reject a NULL aiocbp) before calling the internal functions, while
lio_listio() calls the internal functions directly to preserve its
own lio_link setup.  For entries that are not part of a batch,
lio_listio() self-initializes lio_link instead.

Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
aio_fsync()/aio_read()/aio_write()/lio_listio() initialized
aiocbp->lio_link with list_initialize(), which makes the node
self-referential (prev = next = &node).  aio_signal() tests
list_in_list(&lio_link) to detect lio_listio batches, so it wrongly
entered the lio_listio completion path for every standalone AIO
operation and notified through the uninitialized
lio_sigevent/lio_sigwork.

With CONFIG_SIG_EVTHREAD=y, garbage lio_sigevent.sigev_notify ==
SIGEV_THREAD caused nxsig_notification() to queue &lio_sigwork.work
onto the low-priority work queue with garbage func/value.  After the
aiocb was freed, the dangling work_s was dispatched with worker=NULL,
crashing in work_dispatch().

Fix: initialize lio_link with list_clear_node() (prev = next = NULL)
so list_in_list() returns false for non-lio_listio operations and
aio_signal() skips the lio_listio path.

While there, reject a NULL aiocbp in aio_fsync(): POSIX Issue 6 no
longer defines a NULL special case, and the old DEBUGASSERT() panicked
debug builds.

Co-developed-by: dengwenqi <dengwenqi@xiaomi.com>
Co-developed-by: fangxinyong <fangxinyong@xiaomi.com>
Signed-off-by: fangxinyong <fangxinyong@xiaomi.com>
Signed-off-by: dengwenqi <dengwenqi@xiaomi.com>
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
@raiden00pl

Copy link
Copy Markdown
Member

LTP submits 256 AIO requests, but the PR introduces a default limit of 10 (FS_AIO_LISTIO_MAX), so the default will always fails for LTP

@xiaoxiang781216
xiaoxiang781216 dismissed stale reviews from raiden00pl and jerpelea via 94fd17e September 14, 2026 13:05
@xiaoxiang781216

Copy link
Copy Markdown
Contributor Author

LTP submits 256 AIO requests, but the PR introduces a default limit of 10 (FS_AIO_LISTIO_MAX), so the default will always fails for LTP

fixed.

The new CONFIG_FS_AIO_LISTIO_MAX option defaults to 10 and lio_listio()
now rejects nent > {AIO_LISTIO_MAX} with EINVAL.  The LTP release pinned
by apps/testing/ltp (20230516) submits 256 requests in a single batch from
conformance/interfaces/lio_listio/2-1.c, so ltp_interfaces_lio_listio_2_1
now fails on every configuration that enables CONFIG_TESTING_LTP
(sim:citest, rv-virt:citest, sim:posix_test):

  lio_listio/2-1.c Error at lio_listio() 22: Invalid argument

The EINVAL check itself is required by POSIX, so keep it and raise the
default instead; the limit no longer costs memory because the requests are
linked through the aiocb's own lio_link.

While here, keep _POSIX_AIO_LISTIO_MAX at its POSIX-mandated value of 2
and let AIO_LISTIO_MAX carry the configurable implementation limit.

Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
@acassis
acassis merged commit d5d134b into apache:master Sep 14, 2026
79 of 91 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: File System File System issues Size: L The size of the change in this PR is large

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants